core/mcp/mcp.go (1)

118-128: Guard against nil entries in ClientConfigs to avoid startup panics.

With pointer-based configs, a nil entry will panic when accessing clientConfig.Name or inside AddClient. Add a nil check and only wg.Add(1) for valid configs.

🐛 Proposed fix

 	if len(config.ClientConfigs) > 0 {
-		// Add clients in parallel
-		wg := sync.WaitGroup{}
-		wg.Add(len(config.ClientConfigs))
-		for _, clientConfig := range config.ClientConfigs {
+		// Add clients in parallel
+		wg := sync.WaitGroup{}
+		for _, clientConfig := range config.ClientConfigs {
+			if clientConfig == nil {
+				logger.Warn("%s Skipping nil MCP client config", MCPLogPrefix)
+				continue
+			}
+			wg.Add(1)
 			go func(clientConfig *schemas.MCPClientConfig) {
 				defer wg.Done()
 				if err := manager.AddClient(clientConfig); err != nil {
 					logger.Warn("%s Failed to add MCP client %s: %v", MCPLogPrefix, clientConfig.Name, err)
 				}
 			}(clientConfig)
 		}
 		wg.Wait()
 	}

core/mcp/toolsync.go (1)

224-233: Add a nil guard for pointer-based configs.

With the new pointer signature, a nil config would panic. A small guard makes the function safe and preserves default behavior.

🐛 Suggested fix

 func ResolveToolSyncInterval(clientConfig *schemas.MCPClientConfig, globalInterval time.Duration) time.Duration {
+	if clientConfig == nil {
+		if globalInterval > 0 {
+			return globalInterval
+		}
+		return DefaultToolSyncInterval
+	}
 	// Per-client explicitly disabled (negative value)
 	if clientConfig.ToolSyncInterval < 0 {
 		return 0 // Disabled for this client
 	}

core/bifrost.go (1)

2803-2824: Rename EditMCPClient to UpdateMCPClient to match the new API.

The comment and PR summary indicate UpdateMCPClient, but the exported method is still EditMCPClient, which will break updated call sites and leave the public API inconsistent.

🐛 Proposed fix

-func (bifrost *Bifrost) EditMCPClient(id string, updatedConfig *schemas.MCPClientConfig) error {
+func (bifrost *Bifrost) UpdateMCPClient(id string, updatedConfig *schemas.MCPClientConfig) error {
 	if bifrost.mcpManager == nil {
 		return fmt.Errorf("MCP is not configured in this Bifrost instance")
 	}

 	return bifrost.mcpManager.EditClient(id, updatedConfig)
 }

core/mcp/clientmanager.go (1)

75-112: Initialize ConnectionInfo in placeholders to avoid nil deref.

connectToMCPClient assumes ConnectionInfo is non-nil (Line 442). The placeholders created in AddClient / AddClientInMemory omit it, which will panic on the first connect attempt.

🐛 Proposed fix

 	m.clientMap[config.ID] = &schemas.MCPClientState{
 		Name:            config.Name,
 		ExecutionConfig: config,
 		ToolMap:         make(map[string]schemas.ChatTool),
 		ToolNameMapping: make(map[string]string),
+		ConnectionInfo: &schemas.MCPClientConnectionInfo{
+			Type: config.ConnectionType,
+		},
 	}
@@
 	m.clientMap[config.ID] = &schemas.MCPClientState{
 		Name:            config.Name,
 		ExecutionConfig: config,
 		ToolMap:         make(map[string]schemas.ChatTool),
 		ToolNameMapping: make(map[string]string),
+		ConnectionInfo: &schemas.MCPClientConnectionInfo{
+			Type: config.ConnectionType,
+		},
 	}

Also applies to: 122-159

transports/bifrost-http/lib/config.go (1)

2958-2977: Redaction currently mutates the in-memory config.

configCopy := config is a shallow copy of the pointer, so redaction overwrites the live config (secrets become permanently redacted). Return a value copy instead.

🛠️ Proposed fix

 func (c *Config) RedactMCPClientConfig(config *schemas.MCPClientConfig) *schemas.MCPClientConfig {
-	// Create a shallow copy
-	configCopy := config
+	if config == nil {
+		return nil
+	}
+	// Create a value copy
+	configCopy := *config
 
 	// Redact connection string if present
 	if config.ConnectionString != nil {
 		configCopy.ConnectionString = config.ConnectionString.Redacted()
 	}
@@
 	}
 
-	return configCopy
+	return &configCopy
 }

framework/configstore/rdb.go (2)

795-822: Return the global MCP tool sync interval in the response.

clientConfig.MCPToolSyncInterval is persisted but not propagated into schemas.MCPConfig.ToolSyncInterval, so a configured global interval is ignored at runtime.

🐛 Proposed fix

 	return &schemas.MCPConfig{
 		ClientConfigs:     clientConfigs,
 		ToolManagerConfig: &toolManagerConfig,
+		ToolSyncInterval:  time.Duration(clientConfig.MCPToolSyncInterval) * time.Minute,
 	}, nil

---

889-903: Persist ToolPricing on create.

ToolPricing from the schema isn't mapped into the table model, so new clients drop pricing metadata.

🐛 Proposed fix

 		dbClient := tables.TableMCPClient{
 			ClientID:           clientConfigCopy.ID,
 			Name:               clientConfigCopy.Name,
 			IsCodeModeClient:   clientConfigCopy.IsCodeModeClient,
 			ConnectionType:     string(clientConfigCopy.ConnectionType),
 			ConnectionString:   clientConfigCopy.ConnectionString,
 			StdioConfig:        clientConfigCopy.StdioConfig,
 			AuthType:           string(clientConfigCopy.AuthType),
 			OauthConfigID:      clientConfigCopy.OauthConfigID,
 			ToolsToExecute:     clientConfigCopy.ToolsToExecute,
 			ToolsToAutoExecute: clientConfigCopy.ToolsToAutoExecute,
 			Headers:            clientConfigCopy.Headers,
+			ToolPricing:        clientConfigCopy.ToolPricing,
 			IsPingAvailable:    clientConfigCopy.IsPingAvailable,
 			ToolSyncInterval:   int(clientConfigCopy.ToolSyncInterval.Minutes()),
 		}

core/bifrost.go (2)

2740-2769: Guard against nil MCP client config.

With pointer-based configs, a nil input will likely panic downstream in AddClient. Return a clear error early.

🛠️ Suggested fix

 func (bifrost *Bifrost) AddMCPClient(config *schemas.MCPClientConfig) error {
+	if config == nil {
+		return fmt.Errorf("config is required")
+	}
 	if bifrost.mcpManager == nil {
 		// Use sync.Once to ensure thread-safe initialization
 		bifrost.mcpInitOnce.Do(func() {

---

2804-2826: Rename the exported method to UpdateMCPClient (currently still EditMCPClient).

Comments/examples indicate UpdateMCPClient, but the function name remains EditMCPClient. If call sites were updated as the PR summary suggests, this will fail to compile and leaves the public API inconsistent.

🛠️ Suggested fix

-// UpdateMCPClient updates the MCP client.
+// UpdateMCPClient updates the MCP client.
 // This allows for dynamic MCP client tool management at runtime.
@@
-//	err := bifrost.UpdateMCPClient("my-mcp-client-id", schemas.MCPClientConfig{
+//	err := bifrost.UpdateMCPClient("my-mcp-client-id", schemas.MCPClientConfig{
 //	    Name:           "my-mcp-client-name",
 //	    ToolsToExecute: []string{"tool1", "tool2"},
 //	})
-func (bifrost *Bifrost) EditMCPClient(id string, updatedConfig *schemas.MCPClientConfig) error {
+func (bifrost *Bifrost) UpdateMCPClient(id string, updatedConfig *schemas.MCPClientConfig) error {
+	if updatedConfig == nil {
+		return fmt.Errorf("updatedConfig is required")
+	}
 	if bifrost.mcpManager == nil {
 		return fmt.Errorf("MCP is not configured in this Bifrost instance")
 	}
 
 	return bifrost.mcpManager.EditClient(id, updatedConfig)
 }

framework/configstore/rdb.go (1)

851-873: Guard nil configs and persist ToolPricing on create.

CreateMCPClientConfig and UpdateMCPClientConfig dereference pointer inputs without a nil check, which can panic. Also, ToolPricing is not persisted during create, so pricing data gets dropped.

🛠️ Proposed fix

 func (s *RDBConfigStore) CreateMCPClientConfig(ctx context.Context, clientConfig *schemas.MCPClientConfig) error {
+	if clientConfig == nil {
+		return fmt.Errorf("clientConfig cannot be nil")
+	}
 	return s.db.Transaction(func(tx *gorm.DB) error {
 		// Create a deep copy to avoid modifying the original
 		clientConfigCopy, err := deepCopy(*clientConfig)
 		if err != nil {
 			return err
 		}
@@
 		dbClient := tables.TableMCPClient{
 			ClientID:           clientConfigCopy.ID,
 			Name:               clientConfigCopy.Name,
 			IsCodeModeClient:   clientConfigCopy.IsCodeModeClient,
 			ConnectionType:     string(clientConfigCopy.ConnectionType),
 			ConnectionString:   clientConfigCopy.ConnectionString,
 			StdioConfig:        clientConfigCopy.StdioConfig,
 			AuthType:           string(clientConfigCopy.AuthType),
 			OauthConfigID:      clientConfigCopy.OauthConfigID,
 			ToolsToExecute:     clientConfigCopy.ToolsToExecute,
 			ToolsToAutoExecute: clientConfigCopy.ToolsToAutoExecute,
 			Headers:            clientConfigCopy.Headers,
 			IsPingAvailable:    clientConfigCopy.IsPingAvailable,
 			ToolSyncInterval:   int(clientConfigCopy.ToolSyncInterval.Minutes()),
+			ToolPricing:        clientConfigCopy.ToolPricing,
 		}
@@
 func (s *RDBConfigStore) UpdateMCPClientConfig(ctx context.Context, id string, clientConfig *tables.TableMCPClient) error {
+	if clientConfig == nil {
+		return fmt.Errorf("clientConfig cannot be nil")
+	}
 	return s.db.Transaction(func(tx *gorm.DB) error {
 		// Find existing client
 		var existingClient tables.TableMCPClient

Also applies to: 882-895

core/mcp/clientmanager.go (1)

431-453: Initialize ConnectionInfo before dereferencing.

When a placeholder entry exists (created by AddClient / AddClientInMemory), ConnectionInfo can be nil. existingClient.ConnectionInfo.Type = ... will panic. Initialize it (or skip this write) before use.

🐛 Proposed fix

 if existingClient, exists := m.clientMap[config.ID]; exists {
 	// Client entry exists from config, check for existing connection, if it does then close
 	if existingClient.CancelFunc != nil {
 		existingClient.CancelFunc()
 		existingClient.CancelFunc = nil
 	}
 	if existingClient.Conn != nil {
 		existingClient.Conn.Close()
 	}
 	// Update connection type for this connection attempt
-	existingClient.ConnectionInfo.Type = config.ConnectionType
+	if existingClient.ConnectionInfo == nil {
+		existingClient.ConnectionInfo = &schemas.MCPClientConnectionInfo{}
+	}
+	existingClient.ConnectionInfo.Type = config.ConnectionType
 }

transports/bifrost-http/lib/config.go (2)

2958-2977: Redaction mutates live config due to shallow copy.

configCopy := config keeps the same pointer, so redaction overwrites the stored secrets in memory. Create a value copy before redacting to avoid corrupting runtime config.

🛠️ Proposed fix

 func (c *Config) RedactMCPClientConfig(config *schemas.MCPClientConfig) *schemas.MCPClientConfig {
-	// Create a shallow copy
-	configCopy := config
+	if config == nil {
+		return nil
+	}
+	// Create a value copy
+	configCopy := *config
+	redacted := &configCopy
 
 	// Redact connection string if present
 	if config.ConnectionString != nil {
-		configCopy.ConnectionString = config.ConnectionString.Redacted()
+		redacted.ConnectionString = config.ConnectionString.Redacted()
 	}
 
 	// Redact Header values if present
 	if config.Headers != nil {
-		configCopy.Headers = make(map[string]schemas.EnvVar, len(config.Headers))
+		redacted.Headers = make(map[string]schemas.EnvVar, len(config.Headers))
 		for header, value := range config.Headers {
-			configCopy.Headers[header] = *value.Redacted()
+			redacted.Headers[header] = *value.Redacted()
 		}
 	}
 
-	return configCopy
+	return redacted
 }

---

2946-2954: In-memory MCP config drops updates for new fields.

After a successful update, only name/flags/headers/tools/tool_pricing are copied back to c.MCPConfig. Fields like IsPingAvailable and ToolSyncInterval (sent by the handler) are silently discarded, so future reads/redaction use stale values. Either persist these fields here or explicitly reject edits to them.

🛠️ Proposed fix

 c.MCPConfig.ClientConfigs[configIndex].Name = updatedConfig.Name
 c.MCPConfig.ClientConfigs[configIndex].IsCodeModeClient = updatedConfig.IsCodeModeClient
 c.MCPConfig.ClientConfigs[configIndex].Headers = updatedConfig.Headers
 c.MCPConfig.ClientConfigs[configIndex].ToolsToExecute = updatedConfig.ToolsToExecute
 c.MCPConfig.ClientConfigs[configIndex].ToolsToAutoExecute = updatedConfig.ToolsToAutoExecute
 c.MCPConfig.ClientConfigs[configIndex].ToolPricing = updatedConfig.ToolPricing
+ c.MCPConfig.ClientConfigs[configIndex].IsPingAvailable = updatedConfig.IsPingAvailable
+ c.MCPConfig.ClientConfigs[configIndex].ToolSyncInterval = updatedConfig.ToolSyncInterval

core/mcp/mcp.go (1)

122-128: Guard against nil client configs before dereferencing.

With ClientConfigs now as a pointer slice, a nil entry would panic on clientConfig.Name or inside AddClient. Consider skipping nil configs (or validating earlier) so initialization doesn’t crash.

🛠️ Suggested defensive guard

for _, clientConfig := range config.ClientConfigs {
	go func(clientConfig *schemas.MCPClientConfig) {
		defer wg.Done()
+		if clientConfig == nil {
+			logger.Warn("%s Skipping nil MCP client config", MCPLogPrefix)
+			return
+		}
		if err := manager.AddClient(clientConfig); err != nil {
			logger.Warn("%s Failed to add MCP client %s: %v", MCPLogPrefix, clientConfig.Name, err)
		}
	}(clientConfig)
}

core/mcp/toolsync.go (1)

225-233: Handle nil clientConfig to avoid a panic.

ResolveToolSyncInterval now accepts a pointer; if nil slips through, dereferencing ToolSyncInterval will panic. Add a guard to fall back to the global/default interval.

🛠️ Suggested guard

func ResolveToolSyncInterval(clientConfig *schemas.MCPClientConfig, globalInterval time.Duration) time.Duration {
+	if clientConfig == nil {
+		if globalInterval > 0 {
+			return globalInterval
+		}
+		return DefaultToolSyncInterval
+	}
	// Per-client explicitly disabled (negative value)
	if clientConfig.ToolSyncInterval < 0 {
		return 0 // Disabled for this client
	}

framework/configstore/rdb.go (2)

851-873: Persist ToolPricing on create

ToolPricing is mapped on read and update, but it’s not persisted when creating a new MCP client. This drops pricing from config.json/UI on initial insert.

🐛 Proposed fix

 dbClient := tables.TableMCPClient{
 	ClientID:           clientConfigCopy.ID,
 	Name:               clientConfigCopy.Name,
 	IsCodeModeClient:   clientConfigCopy.IsCodeModeClient,
 	ConnectionType:     string(clientConfigCopy.ConnectionType),
 	ConnectionString:   clientConfigCopy.ConnectionString,
 	StdioConfig:        clientConfigCopy.StdioConfig,
 	AuthType:           string(clientConfigCopy.AuthType),
 	OauthConfigID:      clientConfigCopy.OauthConfigID,
 	ToolsToExecute:     clientConfigCopy.ToolsToExecute,
 	ToolsToAutoExecute: clientConfigCopy.ToolsToAutoExecute,
 	Headers:            clientConfigCopy.Headers,
 	IsPingAvailable:    clientConfigCopy.IsPingAvailable,
 	ToolSyncInterval:   int(clientConfigCopy.ToolSyncInterval.Minutes()),
+	ToolPricing:        clientConfigCopy.ToolPricing,
 }

---

919-946: Fix IsPingAvailable preservation in updates

The guard if clientConfigCopy.IsPingAvailable { clientConfigCopy.IsPingAvailable = true } is a no-op. The update map always writes is_ping_available unconditionally, and mergeMCPRedactedValues does not preserve it from the existing client (unlike ConnectionString and Headers). When an API request omits is_ping_available, it unmarshals to false, inadvertently overwriting the existing value and disabling ping. Preserve the existing value when not explicitly set in the request, similar to how redacted fields are handled—either by extending mergeMCPRedactedValues or by excluding the field from the updates map when it's not explicitly provided.

core/bifrost.go (2)

2741-2753: Update AddMCPClient example to use pointer.

The signature now takes *schemas.MCPClientConfig (Line 2746), but the example still shows a value.

✏️ Doc fix

-//	err := bifrost.AddMCPClient(schemas.MCPClientConfig{
+//	err := bifrost.AddMCPClient(&schemas.MCPClientConfig{
 //	    Name: "my-mcp-client",
 //	    ConnectionType: schemas.MCPConnectionTypeHTTP,
 //	    ConnectionString: &url,
 //	})

---

2810-2832: Public API name mismatch: method is EditMCPClient but documentation references UpdateMCPClient.

The method at line 2826 is named EditMCPClient, but the docstring example (line 2822) calls UpdateMCPClient. This creates an inconsistency with the public API: the higher-level wrappers in Config and BifrostHTTPServer expose UpdateMCPClient, while the core Bifrost type uses EditMCPClient.

There is one internal call site that directly uses EditMCPClient (transports/bifrost-http/lib/config.go:2914). Align the core type with the public API naming by either renaming to UpdateMCPClient or adding a wrapper method.

transports/bifrost-http/lib/config.go (3)

851-863: Add nil guards when iterating MCP client pointers.
ClientConfigs is now []*MCPClientConfig, so a nil entry will panic during merge or lookup. Guard before dereferencing to keep startup/runtime resilient.

🔧 Suggested fix

for _, newClientConfig := range tempMCPConfig.ClientConfigs {
+	if newClientConfig == nil {
+		continue
+	}
	found := false
	for _, existingClientConfig := range mcpConfig.ClientConfigs {
+		if existingClientConfig == nil {
+			continue
+		}
		if (newClientConfig.ID != "" && existingClientConfig.ID == newClientConfig.ID) ||
			(newClientConfig.Name != "" && existingClientConfig.Name == newClientConfig.Name) {
			found = true
			break
		}
	}
	if !found {
		clientConfigsToAdd = append(clientConfigsToAdd, newClientConfig)
	}
}

for _, clientConfig := range c.MCPConfig.ClientConfigs {
+	if clientConfig == nil {
+		continue
+	}
	if clientConfig.ID == id {
		return clientConfig, nil
	}
}

Also applies to: 2768-2771

---

1483-1496: ToolPricing is dropped when converting schema → table.
convertSchemasMCPClientConfigToTable doesn’t map ToolPricing, so persisted pricing can be lost and MCP catalog sync becomes inconsistent.

🔧 Suggested fix

return &configstoreTables.TableMCPClient{
	ClientID:           clientConfig.ID,
	Name:               clientConfig.Name,
	IsCodeModeClient:   clientConfig.IsCodeModeClient,
	ConnectionType:     string(clientConfig.ConnectionType),
	ConnectionString:   clientConfig.ConnectionString,
	StdioConfig:        clientConfig.StdioConfig,
	ToolsToExecute:     clientConfig.ToolsToExecute,
	ToolsToAutoExecute: clientConfig.ToolsToAutoExecute,
	Headers:            clientConfig.Headers,
	AuthType:           string(clientConfig.AuthType),
	OauthConfigID:      clientConfig.OauthConfigID,
+	ToolPricing:        clientConfig.ToolPricing,
}

---

2958-2977: Redaction mutates the original MCP client config.
configCopy := config keeps the same pointer, so redaction overwrites the stored secrets. This can permanently lose connection strings/headers in memory.

🔧 Suggested fix

func (c *Config) RedactMCPClientConfig(config *schemas.MCPClientConfig) *schemas.MCPClientConfig {
-	// Create a shallow copy
-	configCopy := config
+	if config == nil {
+		return nil
+	}
+	// Create a copy so we don't mutate the original
+	configCopy := *config

	// Redact connection string if present
	if config.ConnectionString != nil {
		configCopy.ConnectionString = config.ConnectionString.Redacted()
	}

	// Redact Header values if present
	if config.Headers != nil {
		configCopy.Headers = make(map[string]schemas.EnvVar, len(config.Headers))
		for header, value := range config.Headers {
			configCopy.Headers[header] = *value.Redacted()
		}
	}

-	return configCopy
+	return &configCopy
}

core/bifrost.go (2)

2746-2776: AddMCPClient should reject nil configs.
The new pointer signature allows nil, which can panic or misbehave downstream; fail fast. Also update the example to pass a pointer.

🛠️ Suggested fix

 func (bifrost *Bifrost) AddMCPClient(config *schemas.MCPClientConfig) error {
+	if config == nil {
+		return fmt.Errorf("config is required")
+	}
 	if bifrost.mcpManager == nil {
 		// Use sync.Once to ensure thread-safe initialization

---

2810-2832: Rename wrapper to UpdateMCPClient and validate updatedConfig.
Docs/example already use UpdateMCPClient; keeping EditMCPClient will diverge from callers after the refactor. Also guard against nil pointers.

🛠️ Suggested fix

-func (bifrost *Bifrost) EditMCPClient(id string, updatedConfig *schemas.MCPClientConfig) error {
+func (bifrost *Bifrost) UpdateMCPClient(id string, updatedConfig *schemas.MCPClientConfig) error {
+	if updatedConfig == nil {
+		return fmt.Errorf("updatedConfig is required")
+	}
 	if bifrost.mcpManager == nil {
 		return fmt.Errorf("MCP is not configured in this Bifrost instance")
 	}
 
 	return bifrost.mcpManager.EditClient(id, updatedConfig)
 }

framework/configstore/rdb.go (3)

752-822: Propagate global MCPToolSyncInterval into MCPConfig.
GetMCPConfig builds ToolManagerConfig but doesn’t map TableClientConfig.MCPToolSyncInterval, so custom global sync intervals are ignored.

🛠️ Suggested fix

 	return &schemas.MCPConfig{
 		ClientConfigs:     clientConfigs,
+		ToolSyncInterval:  time.Duration(clientConfig.MCPToolSyncInterval) * time.Minute,
 		ToolManagerConfig: &toolManagerConfig,
 	}, nil
 }

---

851-876: CreateMCPClientConfig should reject nil input.
Pointer signature allows nil; deepCopy(*clientConfig) will panic.

🛠️ Suggested fix

 func (s *RDBConfigStore) CreateMCPClientConfig(ctx context.Context, clientConfig *schemas.MCPClientConfig) error {
+	if clientConfig == nil {
+		return fmt.Errorf("clientConfig is required")
+	}
 	return s.db.Transaction(func(tx *gorm.DB) error {

---

882-947: Guard UpdateMCPClientConfig against nil and clarify IsPingAvailable defaulting.
Nil pointers will panic in deepCopy. Also, if clientConfigCopy.IsPingAvailable { clientConfigCopy.IsPingAvailable = true } is a no‑op; if you intended default‑true when omitted, you’ll need a tri‑state (e.g., *bool) or explicit presence info from callers.

🛠️ Suggested fix

 func (s *RDBConfigStore) UpdateMCPClientConfig(ctx context.Context, id string, clientConfig *tables.TableMCPClient) error {
+	if clientConfig == nil {
+		return fmt.Errorf("clientConfig is required")
+	}
 	return s.db.Transaction(func(tx *gorm.DB) error {

transports/bifrost-http/handlers/mcp.go (1)

145-154: Add nil check for clientConfig after retrieval.

If GetMCPClient returns nil without an error (e.g., client not found), passing nil to AddMCPClient could cause unexpected behavior or panics downstream.

🛠️ Proposed fix

 	clientConfig, err := h.store.GetMCPClient(id)
 	if err != nil {
 		SendError(ctx, fasthttp.StatusInternalServerError, fmt.Sprintf("Failed to get MCP client config: %v", err))
 		return
 	}
+	if clientConfig == nil {
+		SendError(ctx, fasthttp.StatusNotFound, "MCP client config not found")
+		return
+	}

 	if err := h.client.AddMCPClient(clientConfig); err != nil {

core/mcp/mcp.go (1)

118-129: Guard against nil client configs to avoid panic.
With []*MCPClientConfig, a nil entry will panic on clientConfig.Name or inside AddClient. Add a nil check and move wg.Add inside the loop.

🐛 Suggested fix

 	if len(config.ClientConfigs) > 0 {
 		// Add clients in parallel
-		wg := sync.WaitGroup{}
-		wg.Add(len(config.ClientConfigs))
+		wg := sync.WaitGroup{}
 		for _, clientConfig := range config.ClientConfigs {
+			if clientConfig == nil {
+				logger.Warn("%s Skipping nil MCP client config", MCPLogPrefix)
+				continue
+			}
+			wg.Add(1)
 			go func(clientConfig *schemas.MCPClientConfig) {
 				defer wg.Done()
 				if err := manager.AddClient(clientConfig); err != nil {
 					logger.Warn("%s Failed to add MCP client %s: %v", MCPLogPrefix, clientConfig.Name, err)
 				}
 			}(clientConfig)
 		}
 		wg.Wait()
 	}

core/mcp/toolsync.go (1)

225-233: Add a nil guard for the pointer-based config.
Now that this accepts *MCPClientConfig, a nil input will panic. Consider a safe fallback to global/default.

🛡️ Suggested update

 func ResolveToolSyncInterval(clientConfig *schemas.MCPClientConfig, globalInterval time.Duration) time.Duration {
+	if clientConfig == nil {
+		if globalInterval > 0 {
+			return globalInterval
+		}
+		return DefaultToolSyncInterval
+	}
 	// Per-client explicitly disabled (negative value)
 	if clientConfig.ToolSyncInterval < 0 {
 		return 0 // Disabled for this client
 	}

core/bifrost.go (1)

2746-2775: Guard against nil MCP client config.

config can be nil now that the API is pointer-based; add a fast-fail to prevent downstream panics.

🐛 Suggested nil guard

func (bifrost *Bifrost) AddMCPClient(config *schemas.MCPClientConfig) error {
+	if config == nil {
+		return fmt.Errorf("mcp client config is required")
+	}
 	if bifrost.mcpManager == nil {
 		// Use sync.Once to ensure thread-safe initialization
 		bifrost.mcpInitOnce.Do(func() {

framework/configstore/rdb.go (2)

795-822: Global MCP tool sync interval is dropped.

GetMCPConfig builds the schema config but never maps MCPToolSyncInterval into MCPConfig.ToolSyncInterval, so custom global sync settings are ignored after reloads.

🐛 Suggested fix

 return &schemas.MCPConfig{
 	ClientConfigs:     clientConfigs,
-	ToolManagerConfig: &toolManagerConfig,
+	ToolManagerConfig: &toolManagerConfig,
+	ToolSyncInterval:  time.Duration(clientConfig.MCPToolSyncInterval) * time.Minute,
 }, nil

---

859-873: ToolPricing isn’t persisted on MCP client creation.

CreateMCPClientConfig omits ToolPricing, so per-tool pricing from config/API will be lost after restart and MCPCatalog won’t reflect it.

🐛 Suggested fix

 dbClient := tables.TableMCPClient{
 	ClientID:           clientConfigCopy.ID,
 	Name:               clientConfigCopy.Name,
 	IsCodeModeClient:   clientConfigCopy.IsCodeModeClient,
 	ConnectionType:     string(clientConfigCopy.ConnectionType),
 	ConnectionString:   clientConfigCopy.ConnectionString,
 	StdioConfig:        clientConfigCopy.StdioConfig,
 	AuthType:           string(clientConfigCopy.AuthType),
 	OauthConfigID:      clientConfigCopy.OauthConfigID,
 	ToolsToExecute:     clientConfigCopy.ToolsToExecute,
 	ToolsToAutoExecute: clientConfigCopy.ToolsToAutoExecute,
 	Headers:            clientConfigCopy.Headers,
 	IsPingAvailable:    clientConfigCopy.IsPingAvailable,
 	ToolSyncInterval:   int(clientConfigCopy.ToolSyncInterval.Minutes()),
+	ToolPricing:        clientConfigCopy.ToolPricing,
 }

transports/bifrost-http/lib/config.go (1)

1483-1513: Conversion helpers drop ToolPricing/ToolSyncInterval/IsPingAvailable.

Both conversions omit fields now present on schemas.MCPClientConfig, so pricing, sync interval, and ping support can be silently lost when persisting or hydrating configs.

🐛 Suggested fix

 func convertSchemasMCPClientConfigToTable(clientConfig *schemas.MCPClientConfig) *configstoreTables.TableMCPClient {
 	return &configstoreTables.TableMCPClient{
 		ClientID:           clientConfig.ID,
 		Name:               clientConfig.Name,
 		IsCodeModeClient:   clientConfig.IsCodeModeClient,
 		ConnectionType:     string(clientConfig.ConnectionType),
 		ConnectionString:   clientConfig.ConnectionString,
 		StdioConfig:        clientConfig.StdioConfig,
 		ToolsToExecute:     clientConfig.ToolsToExecute,
 		ToolsToAutoExecute: clientConfig.ToolsToAutoExecute,
 		Headers:            clientConfig.Headers,
 		AuthType:           string(clientConfig.AuthType),
 		OauthConfigID:      clientConfig.OauthConfigID,
+		IsPingAvailable:    clientConfig.IsPingAvailable,
+		ToolSyncInterval:   int(clientConfig.ToolSyncInterval.Minutes()),
+		ToolPricing:        clientConfig.ToolPricing,
 	}
 }
 
 func convertTablesMCPClientToSchemas(mcpClient configstoreTables.TableMCPClient) schemas.MCPClientConfig {
 	return schemas.MCPClientConfig{
 		ID:                 mcpClient.ClientID,
 		Name:               mcpClient.Name,
 		IsCodeModeClient:   mcpClient.IsCodeModeClient,
 		ConnectionType:     schemas.MCPConnectionType(mcpClient.ConnectionType),
 		ConnectionString:   mcpClient.ConnectionString,
 		StdioConfig:        mcpClient.StdioConfig,
 		ToolsToExecute:     mcpClient.ToolsToExecute,
 		ToolsToAutoExecute: mcpClient.ToolsToAutoExecute,
 		Headers:            mcpClient.Headers,
 		AuthType:           schemas.MCPAuthType(mcpClient.AuthType),
 		OauthConfigID:      mcpClient.OauthConfigID,
+		IsPingAvailable:    mcpClient.IsPingAvailable,
+		ToolSyncInterval:   time.Duration(mcpClient.ToolSyncInterval) * time.Minute,
+		ToolPricing:        mcpClient.ToolPricing,
 	}
 }

framework/configstore/rdb.go (1)

851-878: Add missing ToolPricing field and nil check

The dbClient struct initialization omits ToolPricing, causing per-client pricing to be lost on create. Additionally, line 855 dereferences clientConfig without checking for nil, risking a panic.

✅ Proposed fix

 func (s *RDBConfigStore) CreateMCPClientConfig(ctx context.Context, clientConfig *schemas.MCPClientConfig) error {
+	if clientConfig == nil {
+		return errors.New("clientConfig is nil")
+	}
 	return s.db.Transaction(func(tx *gorm.DB) error {
 		// Create a deep copy to avoid modifying the original
 		clientConfigCopy, err := deepCopy(*clientConfig)
@@
 		dbClient := tables.TableMCPClient{
 			ClientID:           clientConfigCopy.ID,
 			Name:               clientConfigCopy.Name,
@@
 			IsPingAvailable:    clientConfigCopy.IsPingAvailable,
 			ToolSyncInterval:   int(clientConfigCopy.ToolSyncInterval.Minutes()),
+			ToolPricing:        clientConfigCopy.ToolPricing,
 		}

core/schemas/mcp.go (1)

77-85: Implement backward compatibility for JSON field rename id → client_id.

The struct field was renamed from json:"id" to json:"client_id", breaking existing config files and API payloads that send "id". Without a compatibility layer, the ID will unmarshal as empty. Add a custom UnmarshalJSON method to accept both field names, or ensure all callers are updated and document the breaking change.

🔧 Custom UnmarshalJSON implementation

// UnmarshalJSON supports legacy "id" field for backward compatibility.
func (c *MCPClientConfig) UnmarshalJSON(b []byte) error {
	type Alias MCPClientConfig
	var aux struct {
		*Alias
		LegacyID string `json:"id"`
	}
	aux.Alias = (*Alias)(c)
	if err := json.Unmarshal(b, &aux); err != nil {
		return err
	}
	if c.ID == "" && aux.LegacyID != "" {
		c.ID = aux.LegacyID
	}
	return nil
}

core/bifrost.go (2)

2739-2746: Update AddMCPClient example to pass a pointer.
Signature now expects *schemas.MCPClientConfig, but the doc example still shows a value.

🛠️ Suggested doc update

-//	err := bifrost.AddMCPClient(schemas.MCPClientConfig{
+//	err := bifrost.AddMCPClient(bifrost.Ptr(schemas.MCPClientConfig{
 //	    Name: "my-mcp-client",
 //	    ConnectionType: schemas.MCPConnectionTypeHTTP,
-//	    ConnectionString: &url,
-//	})
+//	    ConnectionString: bifrost.Ptr(url),
+//	}))

Based on learnings, prefer Ptr helper for pointer creation in examples.

---

2810-2832: Rename EditMCPClient or fix the UpdateMCPClient docs.
The comment/example now reference UpdateMCPClient, but the method is still EditMCPClient.

🛠️ Suggested fix (rename method)

-func (bifrost *Bifrost) EditMCPClient(id string, updatedConfig *schemas.MCPClientConfig) error {
+func (bifrost *Bifrost) UpdateMCPClient(id string, updatedConfig *schemas.MCPClientConfig) error {

core/mcp/clientmanager.go (1)

237-248: Guard against nil updatedConfig.
Pointer signature can panic at updatedConfig.Name; add an early check for a clearer error.

🛠️ Suggested fix

 func (m *MCPManager) EditClient(id string, updatedConfig *schemas.MCPClientConfig) error {
+	if updatedConfig == nil {
+		return fmt.Errorf("updatedConfig is required")
+	}
 	m.mu.Lock()
 	defer m.mu.Unlock()

core/mcp/toolsync.go (1)

225-242: Add nil check for clientConfig parameter to prevent potential panic.

The function now accepts a pointer but doesn't guard against nil. If a caller passes nil, accessing clientConfig.ToolSyncInterval at line 227 will cause a nil pointer dereference panic. The related function shouldSkipToolForConfig in utils.go has a nil check (lines 206-208).

Suggested fix

 func ResolveToolSyncInterval(clientConfig *schemas.MCPClientConfig, globalInterval time.Duration) time.Duration {
+	// If no client config, use global or default interval
+	if clientConfig == nil {
+		if globalInterval > 0 {
+			return globalInterval
+		}
+		return DefaultToolSyncInterval
+	}
+
 	// Per-client explicitly disabled (negative value)
 	if clientConfig.ToolSyncInterval < 0 {
 		return 0 // Disabled for this client
 	}

core/bifrost.go (2)

2739-2753: Update the AddMCPClient example to pass a pointer.

The signature now expects *schemas.MCPClientConfig, but the example still passes a value.

🛠️ Suggested doc fix

-//	err := bifrost.AddMCPClient(schemas.MCPClientConfig{
+//	err := bifrost.AddMCPClient(&schemas.MCPClientConfig{
 //	    Name: "my-mcp-client",
 //	    ConnectionType: schemas.MCPConnectionTypeHTTP,
 //	    ConnectionString: &url,
 //	})

---

2810-2831: Rename the method to UpdateMCPClient to match the new API name.

The comment/example say UpdateMCPClient, but the exported symbol is still EditMCPClient. If the stack call sites were updated, this will break compilation and the public API contract.

🛠️ Proposed fix

-func (bifrost *Bifrost) EditMCPClient(id string, updatedConfig *schemas.MCPClientConfig) error {
+func (bifrost *Bifrost) UpdateMCPClient(id string, updatedConfig *schemas.MCPClientConfig) error {
 	if bifrost.mcpManager == nil {
 		return fmt.Errorf("MCP is not configured in this Bifrost instance")
 	}

 	return bifrost.mcpManager.EditClient(id, updatedConfig)
 }

Given the stacked PRs, please ensure the rename is aligned across the stack before merge.

framework/configstore/rdb.go (1)

851-873: Persist ToolPricing on create.
CreateMCPClientConfig doesn’t copy ToolPricing into TableMCPClient, so pricing gets dropped on first persist (the update path does persist it).

🔧 Proposed fix

 dbClient := tables.TableMCPClient{
 	ClientID:           clientConfigCopy.ID,
 	Name:               clientConfigCopy.Name,
 	IsCodeModeClient:   clientConfigCopy.IsCodeModeClient,
 	ConnectionType:     string(clientConfigCopy.ConnectionType),
 	ConnectionString:   clientConfigCopy.ConnectionString,
 	StdioConfig:        clientConfigCopy.StdioConfig,
 	AuthType:           string(clientConfigCopy.AuthType),
 	OauthConfigID:      clientConfigCopy.OauthConfigID,
 	ToolsToExecute:     clientConfigCopy.ToolsToExecute,
 	ToolsToAutoExecute: clientConfigCopy.ToolsToAutoExecute,
 	Headers:            clientConfigCopy.Headers,
 	IsPingAvailable:    clientConfigCopy.IsPingAvailable,
 	ToolSyncInterval:   int(clientConfigCopy.ToolSyncInterval.Minutes()),
+	ToolPricing:        clientConfigCopy.ToolPricing,
 }

core/mcp/clientmanager.go (1)

237-275: EditClient should propagate ToolPricing updates.
UpdateMCPClient updates pricing, but EditClient never copies ToolPricing into the live ExecutionConfig, leaving MCP manager state stale (e.g., GetClients() output).

🔧 Proposed fix

 config := client.ExecutionConfig
 config.Name = updatedConfig.Name
 config.Headers = updatedConfig.Headers
 config.ToolsToExecute = updatedConfig.ToolsToExecute
 config.ToolsToAutoExecute = updatedConfig.ToolsToAutoExecute
 config.IsCodeModeClient = updatedConfig.IsCodeModeClient
+config.ToolPricing = updatedConfig.ToolPricing

transports/bifrost-http/lib/config.go (3)

857-869: Guard against nil client entries in pointer slices.
With []*schemas.MCPClientConfig, JSON null entries can appear and will panic on deref in merge or lookup.

🔧 Proposed fix

 for _, newClientConfig := range tempMCPConfig.ClientConfigs {
+	if newClientConfig == nil {
+		continue
+	}
 	found := false
 	for _, existingClientConfig := range mcpConfig.ClientConfigs {
 		if (newClientConfig.ID != "" && existingClientConfig.ID == newClientConfig.ID) ||
 			(newClientConfig.Name != "" && existingClientConfig.Name == newClientConfig.Name) {
@@
 for _, clientConfig := range c.MCPConfig.ClientConfigs {
-	if clientConfig.ID == id {
+	if clientConfig == nil {
+		continue
+	}
+	if clientConfig.ID == id {
 		return clientConfig, nil
 	}
 }

Also applies to: 2774-2776

---

1489-1503: Schema↔table conversion drops ToolPricing/ToolSyncInterval.
convertSchemasMCPClientConfigToTable omits ToolPricing, ToolSyncInterval, and IsPingAvailable, so persistence loses these fields. Consider mirroring them both ways.

🔧 Proposed fix

 func convertSchemasMCPClientConfigToTable(clientConfig *schemas.MCPClientConfig) *configstoreTables.TableMCPClient {
 	return &configstoreTables.TableMCPClient{
 		ClientID:           clientConfig.ID,
 		Name:               clientConfig.Name,
 		IsCodeModeClient:   clientConfig.IsCodeModeClient,
 		ConnectionType:     string(clientConfig.ConnectionType),
 		ConnectionString:   clientConfig.ConnectionString,
 		StdioConfig:        clientConfig.StdioConfig,
 		ToolsToExecute:     clientConfig.ToolsToExecute,
 		ToolsToAutoExecute: clientConfig.ToolsToAutoExecute,
 		Headers:            clientConfig.Headers,
 		AuthType:           string(clientConfig.AuthType),
 		OauthConfigID:      clientConfig.OauthConfigID,
+		IsPingAvailable:    clientConfig.IsPingAvailable,
+		ToolSyncInterval:   int(clientConfig.ToolSyncInterval.Minutes()),
+		ToolPricing:        clientConfig.ToolPricing,
 	}
 }
 
 func convertTablesMCPClientToSchemas(mcpClient configstoreTables.TableMCPClient) schemas.MCPClientConfig {
 	return schemas.MCPClientConfig{
 		ID:                 mcpClient.ClientID,
 		Name:               mcpClient.Name,
 		IsCodeModeClient:   mcpClient.IsCodeModeClient,
 		ConnectionType:     schemas.MCPConnectionType(mcpClient.ConnectionType),
 		ConnectionString:   mcpClient.ConnectionString,
 		StdioConfig:        mcpClient.StdioConfig,
 		ToolsToExecute:     mcpClient.ToolsToExecute,
 		ToolsToAutoExecute: mcpClient.ToolsToAutoExecute,
 		Headers:            mcpClient.Headers,
 		AuthType:           schemas.MCPAuthType(mcpClient.AuthType),
 		OauthConfigID:      mcpClient.OauthConfigID,
+		IsPingAvailable:    mcpClient.IsPingAvailable,
+		ToolSyncInterval:   time.Duration(mcpClient.ToolSyncInterval) * time.Minute,
+		ToolPricing:        mcpClient.ToolPricing,
 	}
 }

---

2967-2988: Handle nil input in RedactMCPClientConfig.
Now that the function accepts a pointer, a nil input will panic.

🔧 Proposed fix

 func (c *Config) RedactMCPClientConfig(config *schemas.MCPClientConfig) *schemas.MCPClientConfig {
+	if config == nil {
+		return nil
+	}
 	// Create an actual copy of the struct (not just a pointer copy)
 	// This prevents modifying the original config when redacting
 	configCopy := *config

core/mcp/toolsync.go (1)

224-234: Guard against nil clientConfig to avoid panic.
The function now accepts a pointer, but it dereferences without a nil check. A nil input will crash the sync manager.

🐛 Suggested fix

 func ResolveToolSyncInterval(clientConfig *schemas.MCPClientConfig, globalInterval time.Duration) time.Duration {
+	if clientConfig == nil {
+		if globalInterval > 0 {
+			return globalInterval
+		}
+		return DefaultToolSyncInterval
+	}
 	// Per-client explicitly disabled (negative value)
 	if clientConfig.ToolSyncInterval < 0 {
 		return 0 // Disabled for this client
 	}

core/bifrost.go (1)

2810-2832: Fix documentation and method name mismatch: UpdateMCPClient vs EditMCPClient.

The documentation comment and example on lines 2810–2822 reference UpdateMCPClient, but the actual exported method (line 2826) is named EditMCPClient. The example code will not compile. Rename the function to UpdateMCPClient and provide a deprecated EditMCPClient alias for the one internal call site at transports/bifrost-http/lib/config.go:2923.

🛠️ Suggested fix

-func (bifrost *Bifrost) EditMCPClient(id string, updatedConfig *schemas.MCPClientConfig) error {
+func (bifrost *Bifrost) UpdateMCPClient(id string, updatedConfig *schemas.MCPClientConfig) error {
 	if bifrost.mcpManager == nil {
 		return fmt.Errorf("MCP is not configured in this Bifrost instance")
 	}
 	return bifrost.mcpManager.EditClient(id, updatedConfig)
 }
+
+// Deprecated: use UpdateMCPClient.
+func (bifrost *Bifrost) EditMCPClient(id string, updatedConfig *schemas.MCPClientConfig) error {
+	return bifrost.UpdateMCPClient(id, updatedConfig)
+}

core/schemas/mcp.go (1)

79-106: Backwards‑compat risk from id → client_id JSON rename.
Persisted MCP client configs that still serialize id will now unmarshal with an empty ID, which can break updates/removals. Consider accepting both fields during a migration window or running a data migration before rollout.

🛠️ Suggested compatibility shim

+// UnmarshalJSON supports legacy "id" alongside "client_id".
+func (c *MCPClientConfig) UnmarshalJSON(b []byte) error {
+	type Alias MCPClientConfig
+	aux := struct {
+		Alias
+		LegacyID string `json:"id"`
+	}{}
+	if err := sonic.Unmarshal(b, &aux); err != nil {
+		return err
+	}
+	*c = MCPClientConfig(aux.Alias)
+	if c.ID == "" && aux.LegacyID != "" {
+		c.ID = aux.LegacyID
+	}
+	return nil
+}

#!/bin/bash
# Check for legacy MCP client config JSON that still uses "id"
rg -n '"id"\s*:\s*".+"' -g '*.json' -g '*.yml' -g '*.yaml' -g '*.toml'
rg -n 'MCPClientConfigJSON|client_configs' -g '*.go' -g '*.sql' -g '*.json'

framework/configstore/rdb.go (3)

795-822: Propagate the global MCPToolSyncInterval into MCPConfig.

The returned MCPConfig no longer sets ToolSyncInterval, so a configured global interval from TableClientConfig is silently dropped and clients fall back to the default. Add the global interval to keep behavior stable across the stack-wide MCP refactor.

🛠️ Proposed fix

 	return &schemas.MCPConfig{
 		ClientConfigs:     clientConfigs,
-		ToolManagerConfig: &toolManagerConfig,
+		ToolManagerConfig: &toolManagerConfig,
+		ToolSyncInterval:  time.Duration(clientConfig.MCPToolSyncInterval) * time.Minute,
 	}, nil

As per coding guidelines, reviewed in stack context.

---

851-873: Persist tool pricing on MCP client creation.

ToolPricing from schemas.MCPClientConfig is not copied into the TableMCPClient, so pricing gets dropped on create even though updates persist it. This breaks MCP pricing for OAuth and non-OAuth creates.

🛠️ Proposed fix

 		dbClient := tables.TableMCPClient{
 			ClientID:           clientConfigCopy.ID,
 			Name:               clientConfigCopy.Name,
 			IsCodeModeClient:   clientConfigCopy.IsCodeModeClient,
@@
 			Headers:            clientConfigCopy.Headers,
 			IsPingAvailable:    clientConfigCopy.IsPingAvailable,
 			ToolSyncInterval:   int(clientConfigCopy.ToolSyncInterval.Minutes()),
+			ToolPricing:        clientConfigCopy.ToolPricing,
 		}

---

899-947: is_ping_available will be reset to false if omitted from requests.

When the update request deserializes into TableMCPClient, any omitted boolean field defaults to false. The mergeMCPRedactedValues function (which preserves sensitive fields like ConnectionString and Headers) does not handle is_ping_available, and the update directly persists the deserialized value without fallback to existingClient.IsPingAvailable. The code at lines 40–42 (if clientConfigCopy.IsPingAvailable { clientConfigCopy.IsPingAvailable = true }) is a no-op that only reinforces true values.

To fix: either switch IsPingAvailable to *bool for optional handling, or add logic in mergeMCPRedactedValues to preserve the existing value when the field is absent from the request.

transports/bifrost-http/handlers/mcp.go (1)

191-258: OAuth pending config drops ping/tool-sync/pricing fields.

In the OAuth path, the pending MCP config omits IsPingAvailable, ToolSyncInterval, and ToolPricing, so these values are lost after OAuth completes. Persist them the same way as non-OAuth create.

🛠️ Proposed fix

 	if req.AuthType == "oauth" {
@@
-		pendingConfig := schemas.MCPClientConfig{
+		toolSyncInterval := 10 * time.Minute
+		if req.ToolSyncInterval != 0 {
+			toolSyncInterval = time.Duration(req.ToolSyncInterval) * time.Minute
+		} else {
+			config, err := h.store.ConfigStore.GetClientConfig(ctx)
+			if err != nil {
+				SendError(ctx, fasthttp.StatusInternalServerError, fmt.Sprintf("failed to get client config: %v", err))
+				return
+			}
+			if config != nil {
+				toolSyncInterval = time.Duration(config.MCPToolSyncInterval) * time.Minute
+			}
+		}
+
+		pendingConfig := schemas.MCPClientConfig{
 			ID:                 req.ClientID,
 			Name:               req.Name,
 			IsCodeModeClient:   req.IsCodeModeClient,
 			ConnectionType:     schemas.MCPConnectionType(req.ConnectionType),
 			ConnectionString:   req.ConnectionString,
 			StdioConfig:        req.StdioConfig,
 			AuthType:           schemas.MCPAuthType(req.AuthType),
 			OauthConfigID:      &flowInitiation.OauthConfigID,
 			ToolsToExecute:     req.ToolsToExecute,
 			ToolsToAutoExecute: req.ToolsToAutoExecute,
 			Headers:            req.Headers,
+			IsPingAvailable:    req.IsPingAvailable,
+			ToolSyncInterval:   toolSyncInterval,
+			ToolPricing:        req.ToolPricing,
 		}

transports/bifrost-http/lib/config.go (1)

857-869: Guard against nil MCP client entries before dereference.

ClientConfigs is now a pointer slice; a null entry in config JSON would panic during merge or lookup.

🛠️ Suggested guard

for _, newClientConfig := range tempMCPConfig.ClientConfigs {
+	if newClientConfig == nil {
+		continue
+	}
 	found := false
 	for _, existingClientConfig := range mcpConfig.ClientConfigs {
+		if existingClientConfig == nil {
+			continue
+		}
 		if (newClientConfig.ID != "" && existingClientConfig.ID == newClientConfig.ID) ||
 			(newClientConfig.Name != "" && existingClientConfig.Name == newClientConfig.Name) {
 			found = true
 			break
 		}
 	}
 	if !found {
 		clientConfigsToAdd = append(clientConfigsToAdd, newClientConfig)
 	}
}

for _, clientConfig := range c.MCPConfig.ClientConfigs {
+	if clientConfig == nil {
+		continue
+	}
 	if clientConfig.ID == id {
 		return clientConfig, nil
 	}
}

Also applies to: 2774-2776

core/mcp/clientmanager.go (1)

75-99: Avoid sharing mutable MCPClientConfig pointers across layers.

These functions store the caller-owned pointer directly in ExecutionConfig. The transport layer also mutates the same config under a different mutex, which can cause data races and inconsistent reads in the MCP manager. Consider cloning the config before storing/connecting so the manager owns its copy.

🛠️ Suggested fix (clone before storing/connecting)

func (m *MCPManager) AddClient(config *schemas.MCPClientConfig) error {
 	if err := validateMCPClientConfig(config); err != nil {
 		return fmt.Errorf("invalid MCP client configuration: %w", err)
 	}

-	// Make a copy of the config to use after unlocking
-	configCopy := config
+	// Make a copy of the config to use after unlocking
+	configCopy := cloneMCPClientConfig(config)

 	m.mu.Lock()

-	if _, ok := m.clientMap[config.ID]; ok {
+	if _, ok := m.clientMap[configCopy.ID]; ok {
 		m.mu.Unlock()
 		return fmt.Errorf("client %s already exists", config.Name)
 	}

 	// Create placeholder entry
-	m.clientMap[config.ID] = &schemas.MCPClientState{
-		Name:            config.Name,
-		ExecutionConfig: config,
+	m.clientMap[configCopy.ID] = &schemas.MCPClientState{
+		Name:            configCopy.Name,
+		ExecutionConfig: configCopy,
 		ToolMap:         make(map[string]schemas.ChatTool),
 		ToolNameMapping: make(map[string]string),
 		ConnectionInfo: &schemas.MCPClientConnectionInfo{
-			Type: config.ConnectionType,
+			Type: configCopy.ConnectionType,
 		},
 	}

 	// Temporarily unlock for the connection attempt
 	m.mu.Unlock()

 	// Connect using the copied config
 	if err := m.connectToMCPClient(configCopy); err != nil {
 		// Re-lock to clean up the failed entry
 		m.mu.Lock()
-		delete(m.clientMap, config.ID)
+		delete(m.clientMap, configCopy.ID)
 		m.mu.Unlock()
 		return fmt.Errorf("failed to connect to MCP client %s: %w", config.Name, err)
 	}

 	return nil
}

func (m *MCPManager) AddClientInMemory(config *schemas.MCPClientConfig) error {
 	if err := validateMCPClientConfig(config); err != nil {
 		return fmt.Errorf("invalid MCP client configuration: %w", err)
 	}

-	// Make a copy of the config to use after unlocking
-	configCopy := config
+	// Make a copy of the config to use after unlocking
+	configCopy := cloneMCPClientConfig(config)

 	m.mu.Lock()

-	if _, ok := m.clientMap[config.ID]; ok {
+	if _, ok := m.clientMap[configCopy.ID]; ok {
 		m.mu.Unlock()
 		return fmt.Errorf("client %s already exists", config.Name)
 	}

 	// Create placeholder entry
-	m.clientMap[config.ID] = &schemas.MCPClientState{
-		Name:            config.Name,
-		ExecutionConfig: config,
+	m.clientMap[configCopy.ID] = &schemas.MCPClientState{
+		Name:            configCopy.Name,
+		ExecutionConfig: configCopy,
 		ToolMap:         make(map[string]schemas.ChatTool),
 		ToolNameMapping: make(map[string]string),
 		ConnectionInfo: &schemas.MCPClientConnectionInfo{
-			Type: config.ConnectionType,
+			Type: configCopy.ConnectionType,
 		},
 	}

 	// Temporarily unlock for the connection attempt
 	m.mu.Unlock()

 	// Connect using the copied config
 	if err := m.connectToMCPClient(configCopy); err != nil {
 		// Re-lock to clean up the failed entry
 		m.mu.Lock()
-		delete(m.clientMap, config.ID)
+		delete(m.clientMap, configCopy.ID)
 		m.mu.Unlock()
 		return fmt.Errorf("failed to connect to MCP client %s: %w", config.Name, err)
 	}

 	return nil
}

Add a small helper near these methods:

func cloneMCPClientConfig(in *schemas.MCPClientConfig) *schemas.MCPClientConfig {
	if in == nil {
		return nil
	}
	out := *in

	if in.ConnectionString != nil {
		cs := *in.ConnectionString
		out.ConnectionString = &cs
	}
	if in.StdioConfig != nil {
		sc := *in.StdioConfig
		sc.Args = append([]string(nil), in.StdioConfig.Args...)
		sc.Envs = append([]string(nil), in.StdioConfig.Envs...)
		out.StdioConfig = &sc
	}
	if in.Headers != nil {
		out.Headers = maps.Clone(in.Headers)
	}
	if in.ToolsToExecute != nil {
		out.ToolsToExecute = append([]string(nil), in.ToolsToExecute...)
	}
	if in.ToolsToAutoExecute != nil {
		out.ToolsToAutoExecute = append([]string(nil), in.ToolsToAutoExecute...)
	}
	if in.ToolPricing != nil {
		out.ToolPricing = maps.Clone(in.ToolPricing)
	}
	return &out
}

Also applies to: 125-149